Reimplement GWT assay plate designer in React#7623
Reimplement GWT assay plate designer in React#7623labkey-jeckels wants to merge 5 commits intodevelopfrom
Conversation
|
👍 |
labkey-alan
left a comment
There was a problem hiding this comment.
I did not do the most thorough review on the React code, since I was only asked to take a cursory look at the code, and the GitHub outage is preventing me from viewing all of the files at this time. I do have two pieces of feedback though:
- Many of the components have really large chunks of TSX code (e.g. GroupTypesPanel). Lots of nested loops and conditional statements. This is a readability mess, and it also makes it significantly harder to test, since we can't test in smaller units of functionality. My recommendation is to split these large chunks of TSX into smaller components.
- There are zero unit tests. For a set of components this size, and this complex, we really should have as much unit test coverage as is reasonable.
Additionally I do have a concern about introducing this code more generally. While I understand the motivation to get rid of the last GWT component, and I would love to see us accomplish that goal, I am not sure that this is the best path forward. We already have a lot of plate related code in our apps, and while this code helps us get rid of GWT, it does not change the fact that we have two different implementations of a UI that is meant to accomplish the same thing. I think we would benefit most from having one UI for our plate code, not two different implementations that are both made with React.
I can take a more thorough look at the code later if wanted.
Rationale
Our assay plate designer is our final GWT UI. This replaces it with a React-based version.
Changes
Tasks 📍
Test Automation